Support more quadratic solvers #2574

pet-mit · 2025-01-13T13:57:32Z

This PR adds the possibility for the user to choose quadratic (QP) solvers other than sirius, dor adequacy problems.
It does so by interfacing Antares QP description (PROBLEME_ANTARES_A_RESOUDRE) to the new OR-Tools MathOpt API.

The new "quadratic-solver" parameter allows the user to switch solver:

setting the value "sirius" will make Antares behave like before, by shunting OR-Tools and calling sirius directly
setting another value will make Antares use OR-Tools via the MathOpt API. For now, only "pdlp" is supported: SCIP seems to be broken (see here) and XPRESS has not been yet added (see here)

Breaking change:

"ortools-solver" parameter has been renamed to "linear-solver"
"solver-parameters" parameter has been renamed to "linear-solver-parameters"
New parameters have been added : "quadratic-solver" and "quadratic-solver-parameters" (note that the last parameter is a placeholder, and that is not used for now)

Signed-off-by: Peter Mitri <[email protected]>

a-zakir · 2025-01-14T10:45:50Z

src/solver/optimisation/opt_pilotage_optimisation_quadratique.cpp

 {
+    if (options.quadraticSolver != "sirius")
+    {
+        const std::string notFound = "Solver " + options.quadraticSolver


I don’t understand this condition

a-zakir · 2025-01-14T10:46:44Z

src/solver/utils/include/antares/solver/utils/ortools_utils.h

@@ -35,23 +35,43 @@

 using namespace operations_research;

+enum SolverClass


Use enum class

Signed-off-by: Peter Mitri <[email protected]>

sonarqubecloud · 2025-01-21T20:28:03Z

Quality Gate passed

Issues
67 New issues
0 Accepted issues

Measures
0 Security Hotspots
47.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

guilpier-code · 2025-01-21T16:20:39Z

docs/developer-guide/CHANGELOG.md

@@ -12,6 +12,7 @@ toc_depth: 2
 * Short term storage costs [ANT-1854] (#2302)
 * Add ts-generation for links [ANT-1084] (#1986)
 * Make it possible to specify the final hydro reservoir level [ANT-1084] (#1521)
+* Add support for more QP solvers [ANT-2546] (#2574)


About QP : don't hesitate to be more explicit and indicate quadratic problem as in the PR's title

guilpier-code · 2025-01-21T16:27:02Z

src/libs/antares/InfoCollection/StudyInfoCollector.cpp

+    std::string linearSolverName = study_.parameters.optOptions.linearSolver;
+    file_content.addItemToSection("study", "linear solver", linearSolverName);
+
+    std::string quadraticSolverName = study_.parameters.optOptions.linearSolver;


About line :

std::string quadraticSolverName = study_.parameters.optOptions.linearSolver;

By any chance, would you mean (instead) :

std::string quadraticSolverName = study_.parameters.optOptions.quadraticSolver;

?

guilpier-code · 2025-01-21T16:41:17Z

src/libs/antares/study/parameters.cpp

+    optOptions.linearSolver = options.optOptions.linearSolver;
+    optOptions.linearSolverParameters = options.optOptions.linearSolverParameters;
+    optOptions.quadraticSolver = options.optOptions.quadraticSolver;
+    optOptions.quadraticSolverParameters = options.optOptions.quadraticSolverParameters;

    // Options that can be set both in command-line and file
    optOptions.solverLogs = options.optOptions.solverLogs || optOptions.solverLogs;


Instead of these 5 lines, it would be better to have :

optOptions = options.optOptions;

But because of the last line, it seems we can't do that.
But we may if we define an overloading operator in struct OptimizationOptions, with a special treatment for attribute solverLogs.
This operator could be "=" or "<<"

guilpier-code · 2025-01-21T16:59:41Z

src/solver/application/application.cpp

@@ -48,7 +48,10 @@ namespace
 {
 void printSolvers()


In the scope of this PR ? (not sure)

printSolvers() :

Should be renamed into : logAllAvailableSolvers()

why do we print to std::cout and not in usual logs ? Does it even make sense ?

Moreover printSolvers is called from handleOptions which has a very non expressive and probably wrong name and, besides printing, does very strange things.

guilpier-code · 2025-01-21T17:03:30Z

src/solver/application/application.cpp

@@ -48,7 +48,10 @@ namespace
 {
 void printSolvers()
 {
-    std::cout << "Available solvers: " << availableOrToolsSolversString() << std::endl;
+    std::cout << "Available linear solvers: "                                        // NOSONAR


// NOSONAR : what do we mean by that ?

guilpier-code · 2025-01-22T15:53:40Z

src/tests/end-to-end/binding_constraints/test_binding_constraints.cpp

@@ -276,6 +278,7 @@ BOOST_FIXTURE_TEST_CASE(On_year_9__RHS_TS_number_4_is_taken_into_account, StudyW

    OutputRetriever output(simulation->rawSimu());
    BOOST_TEST(output.flow(link).hour(0) == 40., tt::tolerance(0.001));
+    BOOST_TEST(output.flow(link).hour(0) == 40., tt::tolerance(0.001));


Duplication here

guilpier-code · 2025-01-22T15:57:32Z

src/tests/src/solver/utils/CMakeLists.txt

+        INCLUDE
+        "${CMAKE_SOURCE_DIR}/solver/utils"


INCLUDE "${CMAKE_SOURCE_DIR}/solver/utils"

is this useful ?
I mean : if we link with Antares::solverUtils, related include directory should be automatic

guilpier-code · 2025-01-22T16:14:06Z

src/tests/src/solver/utils/test_ortools_quadratic_wrapper.cpp

+        problemeAResoudre.Sens = {};
+        problemeAResoudre.IndicesDebutDeLigne = {};


Here we give to some problemeAResoudre's data members an empty value.
We don't need that : these data members are empty by default at creation.

guilpier-code · 2025-01-22T16:31:43Z

src/tests/src/solver/utils/test_ortools_quadratic_wrapper.cpp

+    }
+};
+
+BOOST_AUTO_TEST_SUITE(tests_on_ortools_quadratic_wrapper)


As all your unit tests use the same fixture, you could use :

BOOST_FIXTURE_TEST_SUITE(tests_on_ortools_quadratic_wrapper, QpFixture); BOOST_AUTO_TEST_CASE(test_case1) {...} BOOST_AUTO_TEST_CASE(test_case2) {...} ... BOOST_AUTO_TEST_SUITE_END()

As you can see, no need to pass the fixture for each test case

guilpier-code · 2025-01-22T16:53:35Z

src/tests/src/solver/utils/test_ortools_quadratic_wrapper.cpp

+
+BOOST_FIXTURE_TEST_CASE(solver_not_supported, QpFixture)
+{
+    options.quadraticSolver = "sirius";


When I look at the fixture (the part that does not "check" anything), I wish it could be turned into an class QuadraticProblem.
If this is possible, in unit tests we could have :

BOOST_FIXTURE_TEST_CASE(simple_qp_one_var, QpFixture) { QuadraticProblem QP(/*whatever needed*/); QP.addVariable("x", 0, 1, -0.5, 1); QP.solve(); BOOST_CHECK(QP.success()); BOOST_TEST(QP.solution() == {0.25}); .... }

Instead, we have to manipulate problemeAResoudre things, we don't want to read about as a user.

Support multiple quadratic solvers

6ea9578

Signed-off-by: Peter Mitri <[email protected]>

pull-request-size bot added the size/L label Jan 13, 2025

UI fix + doc

b5e422e

Signed-off-by: Peter Mitri <[email protected]>

pet-mit added the breaking change This PR/issue introduces a breaking change for users label Jan 13, 2025

a-zakir reviewed Jan 14, 2025

View reviewed changes

updates

6511c37

Signed-off-by: Peter Mitri <[email protected]>

pull-request-size bot added size/XL and removed size/L labels Jan 16, 2025

pet-mit added 10 commits January 17, 2025 15:02

fix

271e62c

Signed-off-by: Peter Mitri <[email protected]>

fix

c49ca2e

Signed-off-by: Peter Mitri <[email protected]>

cleanup

d5d90f0

Signed-off-by: Peter Mitri <[email protected]>

fix UI build

be5564e

Signed-off-by: Peter Mitri <[email protected]>

revert submodule

070ea47

Signed-off-by: Peter Mitri <[email protected]>

Merge branch 'develop' into feature/new_quad_solvers

0aa9b80

some sonar fixes

45397d7

Signed-off-by: Peter Mitri <[email protected]>

read dual values. fix logs. add PDLP support.

6760369

Signed-off-by: Peter Mitri <[email protected]>

add comments

085f4cc

Signed-off-by: Peter Mitri <[email protected]>

tests & some refactoring

5b48ed5

Signed-off-by: Peter Mitri <[email protected]>

pull-request-size bot added size/XXL and removed size/XL labels Jan 21, 2025

update doc

81b4998

Signed-off-by: Peter Mitri <[email protected]>

pet-mit marked this pull request as ready for review January 21, 2025 13:29

pet-mit changed the title ~~Support multiple quadratic solvers~~ Support more quadratic solvers Jan 21, 2025

update cucumber test steps

d9d8c42

Signed-off-by: Peter Mitri <[email protected]>

pet-mit requested a review from a team January 21, 2025 13:35

pet-mit added 3 commits January 21, 2025 14:56

rename test suite

c7ac13f

Signed-off-by: Peter Mitri <[email protected]>

print error message

6629576

Signed-off-by: Peter Mitri <[email protected]>

test new ortools release

445d5ec

Signed-off-by: Peter Mitri <[email protected]>

guilpier-code requested changes Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support more quadratic solvers #2574

Support more quadratic solvers #2574

pet-mit commented Jan 13, 2025 •

edited

Loading

a-zakir Jan 14, 2025

a-zakir Jan 14, 2025

sonarqubecloud bot commented Jan 21, 2025

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025

guilpier-code Jan 22, 2025

guilpier-code Jan 22, 2025

guilpier-code Jan 22, 2025

guilpier-code Jan 22, 2025

guilpier-code Jan 22, 2025

		@@ -35,23 +35,43 @@

		using namespace operations_research;

		enum SolverClass

		problemeAResoudre.Sens = {};
		problemeAResoudre.IndicesDebutDeLigne = {};

Support more quadratic solvers #2574

Are you sure you want to change the base?

Support more quadratic solvers #2574

Conversation

pet-mit commented Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 21, 2025

Quality Gate passed

guilpier-code Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

guilpier-code Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

guilpier-code Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

guilpier-code Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pet-mit commented Jan 13, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading

guilpier-code Jan 21, 2025 •

edited

Loading